Skip to content

Conversation

MasterPtato
Copy link
Contributor

No description provided.

Copy link

vercel bot commented Sep 9, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
rivet-cloud Error Error Sep 16, 2025 9:20am
rivet-site Ready Ready Preview Comment Sep 16, 2025 9:20am
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
rivet-studio Ignored Ignored Preview Sep 16, 2025 9:20am

Copy link

claude bot commented Sep 9, 2025

Claude encountered an error —— View job


I'll analyze this and get back to you.

Copy link
Contributor Author

MasterPtato commented Sep 9, 2025


How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link

pkg-pr-new bot commented Sep 9, 2025

Open in StackBlitz

npm i https://pkg.pr.new/rivet-gg/engine/@rivetkit/engine-runner@2891
npm i https://pkg.pr.new/rivet-gg/engine/@rivetkit/engine-runner-protocol@2891
npm i https://pkg.pr.new/rivet-gg/engine/@rivetkit/engine-tunnel-protocol@2891

commit: 7d7900b

@MasterPtato MasterPtato force-pushed the 09-08-fix_configure_runner_config_per_runner_name branch from b3091dc to a5cea8e Compare September 9, 2025 20:27
@MasterPtato MasterPtato marked this pull request as ready for review September 9, 2025 20:27
Copy link

claude bot commented Sep 9, 2025

Claude encountered an error —— View job


I'll analyze this and get back to you.

Comment on lines +81 to +84
let variant_str = serde_json::to_string(&input.config.variant())?;
ctx.op(internal::ops::cache::purge_global::Input {
base_key: format!("namespace.runner_config.{variant_str}.get_global"),
keys: vec![(input.namespace_id, input.name.as_str()).cache_key().into()],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The serialization of input.config.variant() to JSON creates inconsistent cache keys. Since JSON serialization of enums can vary (quotes, formatting), it may not match the string representation used elsewhere in the codebase.

Consider using a deterministic string representation instead:

let variant_str = input.config.variant().to_string();

This approach ensures cache keys remain consistent across the application, preventing unnecessary cache misses and improving cache hit rates.

Suggested change
let variant_str = serde_json::to_string(&input.config.variant())?;
ctx.op(internal::ops::cache::purge_global::Input {
base_key: format!("namespace.runner_config.{variant_str}.get_global"),
keys: vec![(input.namespace_id, input.name.as_str()).cache_key().into()],
let variant_str = input.config.variant().to_string();
ctx.op(internal::ops::cache::purge_global::Input {
base_key: format!("namespace.runner_config.{variant_str}.get_global"),
keys: vec![(input.namespace_id, input.name.as_str()).cache_key().into()],

Spotted by Diamond

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

@MasterPtato MasterPtato force-pushed the 09-08-feat_ns_implement_namespace_updates_global_cache_purge branch from c376b93 to 8b5ef7d Compare September 9, 2025 20:30
@MasterPtato MasterPtato force-pushed the 09-08-fix_configure_runner_config_per_runner_name branch from a5cea8e to 6b11630 Compare September 9, 2025 20:30
Copy link

claude bot commented Sep 9, 2025

Claude encountered an error —— View job


I'll analyze this and get back to you.

Copy link

claude bot commented Sep 9, 2025

Claude encountered an error —— View job


I'll analyze this and get back to you.

Comment on lines +115 to +124
let for_serverless = txs
.exists(
&namespace::keys::RunnerConfigByVariantKey::new(
namespace_id,
namespace::keys::RunnerConfigVariant::Serverless,
input.runner_name_selector.clone(),
),
SERIALIZABLE,
)
.await?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Database key construction uses wrong namespace. The code creates a namespace::keys::RunnerConfigByVariantKey but uses it with txs which is in the pegboard subspace, not the namespace subspace. This will cause the key lookup to fail since it's looking in the wrong database subspace.

Spotted by Diamond

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

This was referenced Sep 9, 2025
@MasterPtato MasterPtato force-pushed the 09-08-fix_configure_runner_config_per_runner_name branch from 6b11630 to 88d706e Compare September 11, 2025 21:33
@MasterPtato MasterPtato force-pushed the 09-08-feat_ns_implement_namespace_updates_global_cache_purge branch from 8b5ef7d to 4976f94 Compare September 11, 2025 21:33
Copy link

claude bot commented Sep 11, 2025

Claude encountered an error —— View job


I'll analyze this and get back to you.

Copy link

claude bot commented Sep 11, 2025

Claude encountered an error —— View job


I'll analyze this and get back to you.

@NathanFlurry NathanFlurry force-pushed the 09-08-fix_configure_runner_config_per_runner_name branch from c5ce959 to 7679f25 Compare September 14, 2025 20:00
@NathanFlurry NathanFlurry force-pushed the 09-08-feat_ns_implement_namespace_updates_global_cache_purge branch from 02c9438 to 21d608d Compare September 14, 2025 20:00
Copy link

claude bot commented Sep 14, 2025

Claude encountered an error —— View job


I'll analyze this and get back to you.

Copy link

claude bot commented Sep 14, 2025

Claude encountered an error —— View job


I'll analyze this and get back to you.

Copy link

claude bot commented Sep 15, 2025

Claude encountered an error —— View job


I'll analyze this and get back to you.

@MasterPtato MasterPtato force-pushed the 09-08-fix_configure_runner_config_per_runner_name branch from c5ce959 to 7d7900b Compare September 15, 2025 18:04
@MasterPtato MasterPtato force-pushed the 09-08-feat_ns_implement_namespace_updates_global_cache_purge branch from 02c9438 to 3df4e46 Compare September 15, 2025 18:04
Copy link

claude bot commented Sep 15, 2025

Claude encountered an error —— View job


I'll analyze this and get back to you.

Copy link

claude bot commented Sep 15, 2025

Claude encountered an error —— View job


I'll analyze this and get back to you.

@MasterPtato MasterPtato mentioned this pull request Sep 15, 2025
@NathanFlurry NathanFlurry force-pushed the 09-08-feat_ns_implement_namespace_updates_global_cache_purge branch from 3df4e46 to 012075b Compare September 16, 2025 08:12
Base automatically changed from 09-08-feat_ns_implement_namespace_updates_global_cache_purge to main September 16, 2025 08:12
@NathanFlurry NathanFlurry force-pushed the 09-08-fix_configure_runner_config_per_runner_name branch from 7d7900b to c2c14ae Compare September 16, 2025 08:13
Copy link

claude bot commented Sep 16, 2025

Claude encountered an error —— View job


I'll analyze this and get back to you.

@NathanFlurry NathanFlurry merged commit a78e100 into main Sep 16, 2025
4 of 11 checks passed
@NathanFlurry NathanFlurry deleted the 09-08-fix_configure_runner_config_per_runner_name branch September 16, 2025 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants